Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: New SSO workflow skeleton #1032

Merged
merged 3 commits into from
Sep 21, 2023
Merged

feat: New SSO workflow skeleton #1032

merged 3 commits into from
Sep 21, 2023

Conversation

marlonkeating
Copy link
Contributor

@marlonkeating marlonkeating commented Sep 15, 2023

Jira Ticket

Placeholder screens for the new SSO Integration workflow.

image

image

image

image

For all changes

  • Ensure adequate tests are in place (or reviewed existing tests cover changes)

Only if submitting a visual change

  • Ensure to attach screenshots
  • Ensure to have UX team confirm screenshots

Remaining Work

  • Change edX Service Provider Metadata button color to Primary
  • Show SAP fields on page 2, which will be selectively hidden later
  • Add selectively hidden metadata fields on page 1
    • Hidden metadata fields unit tests
  • Add back buttons on pages 2+

@codecov
Copy link

codecov bot commented Sep 15, 2023

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.01% 🎉

Comparison is base (f4328dc) 83.01% compared to head (09e4c71) 83.03%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1032      +/-   ##
==========================================
+ Coverage   83.01%   83.03%   +0.01%     
==========================================
  Files         412      412              
  Lines        8849     8857       +8     
  Branches     1819     1821       +2     
==========================================
+ Hits         7346     7354       +8     
  Misses       1464     1464              
  Partials       39       39              
Files Changed Coverage Δ
...mponents/settings/SettingsLMSTab/LMSConfigPage.jsx 100.00% <ø> (ø)
...mponents/settings/SettingsSSOTab/NewSSOStepper.jsx 100.00% <100.00%> (ø)
src/components/test/testUtils.jsx 75.00% <100.00%> (+2.27%) ⬆️

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@alex-sheehan-edx alex-sheehan-edx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A couple thoughts from manual testing, happy to discuss:

  • After the first step, I think we need a back button for all the other steps instead of the cancel button
  • Can we remove the save on cancel language from the cancel modal
  • Can we center the stepper elements to match figma
  • Can you add all the fields to step two (not just the SAP ones)? My thinking is that once we have the config context we can hide/show the sap fields depending on the external idp
  • Can you add the behavior of selecting the idp metadata radios in step one reveal the field (in the case of metadata url) and the file upload (in the case of xml file)? No need for the full file upload, hopefully it can just be a mock implementation?

@@ -37,6 +37,7 @@ const LMSConfigPage = ({
return (
<div>
<FormContextWrapper
workflowTitle="New learning platform integration"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is this getting added to the LMS settings tab?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The FormWorkflow originally had this hard-coded, as it was only used in the LMS config stepper.

@marlonkeating marlonkeating force-pushed the mkeating/ENT-7585 branch 3 times, most recently from d5e5916 to 77eeb1b Compare September 19, 2023 00:28
@marlonkeating marlonkeating force-pushed the mkeating/ENT-7585 branch 2 times, most recently from 6558648 to 1a8cac4 Compare September 19, 2023 23:22
fix: Make service provider metadata button primary color

feat: Add non-SAP fields to SSO Configure page

feat: Show different SSO metadata inputs based on radio selection

test: Add test for SSO metadata input hiding

feat: Add back buttons to sso config workflow

chore: rename function
Copy link
Contributor

@alex-sheehan-edx alex-sheehan-edx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks awesome

@marlonkeating marlonkeating merged commit e5edf15 into master Sep 21, 2023
6 checks passed
@marlonkeating marlonkeating deleted the mkeating/ENT-7585 branch September 21, 2023 21:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants